Skip to content

fix(http): validate Host and Origin for streamable-http and sse transports#382

Merged
feiskyer merged 4 commits into
mainfrom
fix/http-host-origin-validation
Jun 24, 2026
Merged

fix(http): validate Host and Origin for streamable-http and sse transports#382
feiskyer merged 4 commits into
mainfrom
fix/http-host-origin-validation

Conversation

@gossion

@gossion gossion commented Jun 18, 2026

Copy link
Copy Markdown
Member

Summary

  • Browsers can DNS-rebind an attacker-controlled hostname to a victim-local or private AKS-MCP listener and invoke MCP tools under the server-side Azure / Kubernetes identity when OAuth is disabled. This PR closes that path with three layers:
    • New middleware (`internal/server/httpsecurity`) wrapped around `/mcp`, `/sse` and `/message`. Empty allowlist defaults to loopback only; explicit `--allowed-host` / `--trusted-origin` entries match case-insensitive (with optional port). `*` is honored as a reverse-proxy escape valve. Rejections return 403 with a JSON error body and a structured warning log. `/health` stays outside the middleware so liveness/readiness probes are unaffected.
    • Startup-time validator (`internal/config/config.go`) refuses to start with the combination streamable-http or sse + non-loopback bind + OAuth disabled + empty `--allowed-host`. Operators must pick one of (a) loopback bind, (b) `--oauth-enabled`, or (c) `--allowed-host=`. CrashLoopBackOff at deploy time is preferred over a silently exploitable endpoint.
    • Helm surface (`chart/values.yaml`, `chart/templates/deployment.yaml`) exposes `security.allowedHosts` / `security.trustedOrigins`, threaded into container args. Defaults stay empty so OAuth-enabled or loopback-bound deployments are not surprised; default-insecure deployments now fail closed via the validator.

Test plan

  • `go test ./internal/server/... ./internal/config/...` (new `httpsecurity` package, server middleware wiring, ValidateConfig refusal + safe combinations)
  • `make lint` / `make vet` / `make fmt` clean
  • Manual: `./aks-mcp --transport streamable-http --host 0.0.0.0 --port 8000` exits with the DNS-rebinding refusal
  • Manual: a foreign `Host: rebind.example.com` request to a running server returns `403` and emits the structured warning log
  • Manual: `helm template chart/ --set 'security.allowedHosts={aks-mcp.example.com}'` renders `--allowed-host` into the container args

gossion added 2 commits June 18, 2026 11:55
…ports

Browsers can reach the streamable-http and sse endpoints via DNS rebinding
and then invoke MCP tools under the server-side Azure / Kubernetes identity
when OAuth is disabled. Add a transport-agnostic Host/Origin allowlist
middleware in front of /mcp, /sse and /message, plus a startup-time validator
that refuses to bring up a public HTTP listener with no authentication and no
explicit trusted-host allowlist.

- internal/server/httpsecurity: new middleware with loopback-only default
  allowlist, explicit allow entries (case-insensitive, with optional port),
  and a "*" escape valve for trusted reverse-proxy deployments. Empty Origin
  (non-browser callers) is accepted; any non-empty Origin must match the
  allowlist. Rejections return 403 with a JSON error body and a structured
  warning log.
- internal/config: add --allowed-host / --trusted-origin flags and
  ValidateConfig() rejection of "streamable-http|sse + non-loopback bind +
  OAuth disabled + no --allowed-host" so insecure deployments fail closed at
  startup instead of silently exposing tools.
- internal/server: wire the middleware around /mcp, /sse and /message
  (outside OAuth so foreign origins are rejected before any session work).
  /health stays outside the middleware so probes are unaffected.
- chart: expose security.allowedHosts and security.trustedOrigins, threaded
  into deployment args. Defaults are empty so existing OAuth-enabled or
  ingress-fronted deployments are not surprised; insecure-default deployments
  CrashLoopBackOff via the new validator.
- tests: cover the middleware decision matrix, the startup-time validator,
  and confirm the middleware is mounted on both transports.
…default

When OAuth is enabled, a valid bearer token is already required for tool
dispatch and DNS rebinding cannot produce one, so applying the Host/Origin
allowlist would only break legitimate ingress / reverse-proxy deployments
that forward the public hostname. The middleware now defaults to allow-any
in that configuration; operators who still want belt-and-suspenders Host
enforcement can opt back in via --allowed-host / --trusted-origin.
@gossion

gossion commented Jun 18, 2026

Copy link
Copy Markdown
Member Author

Backwards-compatibility impact

This change adds two new flags (--allowed-host, --trusted-origin) and a startup-time validator. Existing users split into two groups:

CLI users (./aks-mcp ...) — no break

  • --host default is unchanged (127.0.0.1, loopback).
  • --allowed-host / --trusted-origin are new optional flags that default to empty.
  • The new validator only refuses to start when all four conditions hold: HTTP transport AND non-loopback host AND OAuth disabled AND no --allowed-host.

Default CLI invocations bind to loopback, so the second condition is false. No command-line changes required, behavior unchanged.

The only CLI invocation that now fails is exactly the DNS-rebinding-vulnerable posture:

./aks-mcp --transport streamable-http --host 0.0.0.0   # refuses to start

The error message offers three fixes: --host 127.0.0.1, --oauth-enabled, or --allowed-host=<your.hostname>.

Helm chart users — breaking change, requires values.yaml update

chart/templates/deployment.yaml hardcodes --host 0.0.0.0 and chart/values.yaml defaults oauth.enabled: false. A default Helm install therefore hits the new validator and the Pod will CrashLoopBackOff on upgrade.

Existing Helm users must pick one of:

# (a) Already using OAuth — nothing to change
oauth:
  enabled: true

# (b) Internal ClusterIP traffic only — declare trusted Host explicitly
security:
  allowedHosts:
    - "aks-mcp.internal.example.com"

# (c) Trust upstream reverse proxy to validate Host
security:
  allowedHosts:
    - "*"

This is the intentional "secure by default" trade-off: the previous default exposed /mcp and /sse to any browser-origin attacker that could DNS-rebind onto a reachable address. The startup-time refusal forces operators to make one explicit choice instead of silently shipping the vulnerability.

Middleware defaults under OAuth

When oauth.enabled: true, the runtime middleware now defaults to allow-any Host/Origin. A valid bearer token is already required for tool dispatch and DNS rebinding cannot produce one, so applying the allowlist by default would only break legitimate ingress / reverse-proxy deployments. Operators who want belt-and-suspenders Host enforcement on top of OAuth can still set --allowed-host / security.allowedHosts to tighten the policy.

@gossion gossion force-pushed the fix/http-host-origin-validation branch from 6572623 to 76b42c3 Compare June 24, 2026 06:15
@gossion gossion requested a review from Copilot June 24, 2026 06:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens AKS-MCP’s HTTP-based transports (streamable-http and SSE) against DNS-rebinding / cross-origin abuse by adding Host/Origin enforcement middleware, startup-time “fail closed” validation for unsafe deployment combinations, and Helm values to configure the new policy. It also refines Azure CLI tool error handling to key off exit codes while treating stderr-only output as a warning.

Changes:

  • Add internal/server/httpsecurity middleware and wire it in front of /mcp, /sse, and /message (excluding /health).
  • Add config flags and ValidateConfig() refusal for publicly reachable HTTP transports when OAuth is disabled and no host allowlist is set.
  • Expose the allowlists through Helm chart values and deployment args.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/server/server.go Wires Host/Origin middleware into streamable-http + SSE endpoints and adds helper methods for testable mounting/default behavior.
internal/server/server_test.go Adds tests that assert middleware is mounted and enforces expected defaults/overrides.
internal/server/httpsecurity/host_origin.go Implements Host/Origin enforcement middleware with loopback defaults and JSON 403 responses.
internal/server/httpsecurity/host_origin_test.go Unit tests for allowlist matching and middleware behavior (Host/Origin, wildcard, loopback).
internal/config/config.go Adds --allowed-host / --trusted-origin parsing and startup-time refusal for unsafe public HTTP binds without auth/allowlist.
internal/config/config_test.go Adds validation tests for unsafe vs safe transport/auth/bind combinations.
internal/components/azapi/handler.go Switches Azure CLI failure detection to ExitCode != 0 and logs stderr as warning when exit code is 0.
internal/components/azapi/handler_test.go Updates mocks for ExitCode and adds coverage for warning-only stderr results and exit code messaging.
chart/values.yaml Adds security.allowedHosts / security.trustedOrigins values and documents startup refusal behavior.
chart/templates/deployment.yaml Threads Helm security values into container args (--allowed-host, --trusted-origin).

Comment thread internal/server/httpsecurity/host_origin.go
Comment thread internal/config/config.go
- isHostAllowed: match allowlist entries that carry an explicit port
  (e.g. "host:8000"), handle IPv6 bracketed forms with/without port,
  and still allow port-less entries to match any port.
- isLoopbackBindHost: use net.ParseIP().IsLoopback() so the whole
  127.0.0.0/8 range and ::1 are recognised, not just 127.0.0.1.
- buildHTTPSecurityMiddleware: when the listener is bound to a
  loopback address and OAuth is off, relax the Origin default to "*"
  while keeping the Host check on the strict loopback-only path. This
  preserves the curl-from-localhost workflow without weakening the
  DNS-rebinding mitigation for non-loopback binds.
- chart/values.yaml: drop the misleading "bind to loopback" option
  (the chart hard-codes 0.0.0.0) and document that allowedHosts
  entries may include a port.
- tests: cover port-bearing and IPv6 bracketed allowlist entries,
  127.0.0.0/8 loopback bind, the new loopback-Origin relaxation, and
  a strict-mode regression guard for non-loopback binds.
- container_test: pass --allowed-host=localhost so the streamable-http
  smoke test satisfies the new fail-closed startup validator.

@feiskyer feiskyer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@feiskyer feiskyer added this pull request to the merge queue Jun 24, 2026
Merged via the queue into main with commit d18f600 Jun 24, 2026
9 checks passed
@feiskyer feiskyer deleted the fix/http-host-origin-validation branch June 24, 2026 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants